Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add the choice of whether to perform certificate verification when connect to ldaps:// #86

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add the choice of whether to perform certificate verification when connect to ldaps:// #86

wants to merge 4 commits into from

Conversation

longping-t
Copy link

@longping-t longping-t commented Apr 19, 2021 •
edited
Loading

When connecting to LDAP server over SSL/TLS(LDAPS) with self-signed certificate, the jenkins server cannot connect to LDAP server.
The error log shows that [Root exception is javax.net.ssl.SSLHandshakeException: sun.security.validator.ValidatorException: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target].

If we can get the cert from LDAP server, we import the cert to trust cacerts in jenkins server. Then the above error can be resolved. But if there are multiple servers, this is a bit troublesome.

Another way is to skip certificate validation.

Add the choice "SSL Verify" in LDAP configuarion for users to choose:

  1. If checked(the default) and ldap server is an ldaps:// style URL, requiring the certificate to be verified.
  2. If unchecked, Jenkins will not verify the server certificate.

rsandell
rsandell requested changes Apr 21, 2021
View reviewed changes
Copy link
Member

@rsandell rsandell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you write some for of test for this?

src/main/java/hudson/security/LDAPSecurityRealm.java Outdated Show resolved Hide resolved
src/main/java/hudson/security/LDAPSecurityRealm.java Outdated Show resolved Hide resolved
src/main/java/jenkins/security/plugins/ldap/LDAPConfiguration.java Outdated Show resolved Hide resolved
rsandell
rsandell requested changes May 17, 2021
View reviewed changes
Copy link
Member

@rsandell rsandell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid force pushing in the future if possible. It makes reviewing harder as I can't see what has changed since I last reviewed.

src/main/java/jenkins/security/plugins/ldap/LDAPConfiguration.java Outdated Show resolved Hide resolved
@longping-t longping-t requested a review from rsandell May 31, 2021 08:55
@wbehrens-on-gh
Copy link

It would also be nice to add some information to the readme about enabling ldaps:// currently even using Google returns 0 results about how to configure it in Jenkins

@Tethik
Copy link

Tethik commented Jul 18, 2022

If unchecked (the default), Jenkins will not verify the server certificate.

It's bad security practice to opt out of certificate validation by default as it opens up for man-in-the-middle attacks. Please leave it on, and allow users to opt out if needed.

@longping-t longping-t requested a review from a team as a code owner September 19, 2022 09:42
@longping-t
Copy link
Author

If unchecked (the default), Jenkins will not verify the server certificate.

It's bad security practice to opt out of certificate validation by default as it opens up for man-in-the-middle attacks. Please leave it on, and allow users to opt out if needed.

I have modified the default case to need to verify the certificate. Thank you for your advice.

@jtnord
Copy link
Member

jtnord commented Mar 20, 2024 •
edited
Loading

I would say this is not something that should be added.
If there is a need to support a self signed cert in 2024 then is there a reason that this cert can not be added to the JVM keystore?

@jtnord
Copy link
Member

jtnord commented Apr 8, 2024 •
edited
Loading

If we can get the cert from LDAP server, we import the cert to trust cacerts in jenkins server. Then the above error can be resolved. But if there are multiple servers, this is a bit troublesome.

FWIW: you can have a common root cert for these servers and then you can just import that root cert, not all of the certs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rsandell rsandell Awaiting requested review from rsandell

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@longping-t @wbehrens-on-gh @Tethik @jtnord @rsandell